Skip to content

feat: [Magic Link Login] add placeholder to invalidEmail to return user email.#1145

Merged
datamweb merged 31 commits intocodeigniter4:developfrom
warcooft:patch-1
Jul 25, 2024
Merged

feat: [Magic Link Login] add placeholder to invalidEmail to return user email.#1145
datamweb merged 31 commits intocodeigniter4:developfrom
warcooft:patch-1

Conversation

@warcooft
Copy link
Copy Markdown
Contributor

@warcooft warcooft commented Jul 17, 2024 •
edited by datamweb
Loading

Description
This pull request improves existing translations by returning the user's email if the corresponding email is not found. This update aims to make communication more engaging and user-friendly. By returning the email value in the error message, it will help users to confirm and correct the email sent because in general users often make typos.

Preview

Screenshot 2024-07-17 at 14 41 47

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added tests needed Pull requests that need tests enhancement New feature or request labels Jul 17, 2024
@kenjis
Copy link
Copy Markdown
Member

kenjis commented Jul 17, 2024

Please fix errors in GitHub Action checks, and add test code.
Reference: https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

@warcooft
Copy link
Copy Markdown
Contributor Author

warcooft commented Jul 18, 2024

@kenjis all language files need to be updated but I don't understand other languages. Should I use english for other files as default language to be translated later by respective authors?

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Jul 18, 2024

@warcooft
See 2dc1252#diff-2b85118f8a629f90163570fda562d9379b8ee25dafc16e8748e7eb892884ddd2

@kenjis kenjis removed the tests needed Pull requests that need tests label Jul 18, 2024
@warcooft
Copy link
Copy Markdown
Contributor Author

warcooft commented Jul 18, 2024

Ahh, OK. I will update later.

warcooft added 18 commits July 18, 2024 13:36
@warcooft
lang: add temp lang
829a327
@warcooft
lang: add temp lang [invalidEmail]
9889424
@warcooft
lang: add temp lang [invalidEmail]
55b3475
@warcooft
lang: add temp lang [invalidEmail]
6e76788
@warcooft
lang: add temp lang [invalidEmail]
0711286
@warcooft
lang: add temp lang
fa0416b
@warcooft
lang: add temp lang [invalidEmail]
a6f196b
@warcooft
lang: add temp lang [invalidEmail]
2c59425
@warcooft
lang: add temp lang [invalidEmail]
6080c95
@warcooft
lang: add temp lang [invalidEmail]
cc5fb61
@warcooft
lang: add temp lang [invalidEmail]
d4fa7dd
@warcooft
lang: add temp lang [invalidEmail]
758cb25
@warcooft
lang: add temp lang [invalidEmail]
2734645
@warcooft
lang: add temp lang [invalidEmail]
b952676
@warcooft
lang: add temp lang [invalidEmail]
6f7e9eb
@warcooft
lang: add temp lang [invalidEmail]
ece0a3b
@warcooft
lang: add temp lang [invalidEmail]
0f7a6ee
@warcooft
'invalidEmail' => '(To be translated) Unable to verify the email addr…
795eda0 
…ess matches the email {0}.',
@warcooft warcooft mentioned this pull request Jul 18, 2024
Merged
5 tasks
kenjis
kenjis reviewed Jul 19, 2024
View reviewed changes
@kenjis kenjis changed the title feat: add placeholder to invalidEmail to return user email. feat: [Magic Link Login] add placeholder to invalidEmail to return user email. Jul 19, 2024
@kenjis
Copy link
Copy Markdown
Member

kenjis commented Jul 19, 2024

Escape this:

shield/src/Views/magic_link_form.php

Line 22 in 1829443

<?= session('errors') ?>

@warcooft
Copy link
Copy Markdown
Contributor Author

warcooft commented Jul 19, 2024

I'm in a meeting now, I'll update again later.

kenjis
kenjis requested changes Jul 20, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Escape this, too.

shield/src/Views/magic_link_form.php

Line 22 in 1829443

<?= session('errors') ?>

@warcooft warcooft requested a review from kenjis July 20, 2024 03:44
@kenjis
Copy link
Copy Markdown
Member

kenjis commented Jul 22, 2024 •
edited
Loading

@warcooft We don't use merge commits in PR branches. See GitHub Actions.
Can you remove it?
How to update your PR branch: https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#updating-your-branch

@warcooft
Copy link
Copy Markdown
Contributor Author

warcooft commented Jul 22, 2024

ups, my bad, okay I'll rebase.

@warcooft
Copy link
Copy Markdown
Contributor Author

warcooft commented Jul 22, 2024

It seems to be fine now, please check it.

kenjis
kenjis reviewed Jul 22, 2024
View reviewed changes
@warcooft
Copy link
Copy Markdown
Contributor Author

warcooft commented Jul 22, 2024

Where can I find that key in the test case?

19) Tests\Language\UkrainianTranslationTest::testAllIncludedLanguageKeysAreTranslated with data set "uk" ('uk')
Failed asserting that the translated language key "Auth.invalidEmail" in "uk" locale differs from the original keys in the main repository.
Failed asserting that an array is empty.

@kenjis
Copy link
Copy Markdown
Member

kenjis commented Jul 23, 2024

Where can I find that key in the test case?

See https://github.com/codeigniter4/shield/pull/1145/files#diff-12beddb17619112c65bd21202ce1fe8bcd291beb594aa101c90f09b174289fc7R28

Failed asserting that the translated language key "Auth.invalidEmail" in "uk" locale differs from the original keys in the main repository.

It is difficult to understand, but it tells "Auth.invalidEmail" in "uk" must be different from en.

@warcooft
Copy link
Copy Markdown
Contributor Author

warcooft commented Jul 23, 2024

I see, I forgot to add something.

warcooft
warcooft commented Jul 23, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@warcooft warcooft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@warcooft warcooft requested a review from kenjis July 23, 2024 06:17
kenjis
kenjis approved these changes Jul 23, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

datamweb
datamweb approved these changes Jul 25, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@datamweb datamweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@warcooft thank you!

Sorry for the delay.

@datamweb datamweb merged commit 032dad9 into codeigniter4:develop Jul 25, 2024
@warcooft warcooft deleted the patch-1 branch July 25, 2024 17:07
@kenjis kenjis mentioned this pull request Aug 11, 2024
Merged
5 tasks
@michalsn michalsn mentioned this pull request Jul 14, 2025
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kenjis kenjis kenjis approved these changes

@datamweb datamweb datamweb approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@warcooft @kenjis @datamweb